Skip to content

tools: backport tools/install.py for headers#4149

Closed
richardlau wants to merge 1 commit intonodejs:v0.12-stagingfrom
richardlau:v0.12-staging
Closed

tools: backport tools/install.py for headers#4149
richardlau wants to merge 1 commit intonodejs:v0.12-stagingfrom
richardlau:v0.12-staging

Conversation

@richardlau
Copy link
Member

@richardlau richardlau commented Dec 4, 2015

Backport the tools/install.py changes from 628a3ab that were missed
when 6fb0b92 backported the corresponding changes to the Makefile to
build the headers only archive.

Fixes #4140

@mscdex mscdex added the tools Issues and PRs related to the tools directory. label Dec 4, 2015
@mscdex
Copy link
Contributor

mscdex commented Dec 4, 2015

Maybe it would be better to target tools: instead of build:?

@rvagg
Copy link
Member

rvagg commented Dec 4, 2015

lgtm, thanks @richardlau

I have no opinion on tools: vs build:

@richardlau
Copy link
Member Author

richardlau commented Dec 4, 2015

I've tested by running HEADERS_ONLY=1 python tools/install.py install 'node-v0.12.10' '/' and inspecting the contents of the created node-v0.12.10 directory.

I tried to run make tar-headers but ran into some problems:

  1. First I see this, which is easily patched around
#NODE_VERSION_IS_RELEASE is set to 0.
Did you remember to update src/node_version.h?
  1. If I patch src/node_version.h for 1) I then get:
find node-v0.12.10/ -type l | xargs rm # annoying on windows
rm: missing operand
Try `rm --help' for more information.
make: *** [node-v0.12.10.tar-headers] Error 123

But I also get the same problem with v4.x.

@richardlau
Copy link
Member Author

richardlau commented Dec 4, 2015

@mscdex I can retarget to tools: if required -- I picked build: because the two commits referenced were targeted at build:.

@richardlau
Copy link
Member Author

richardlau commented Dec 4, 2015

This also affects v0.10 -- Please could someone add a lts-watch-v0.10 label?

@richardlau
tools: backport tools/install.py for headers
4a9a611 
Backport the tools/install.py changes from 628a3ab that were missed
when 6fb0b92 backported the corresponding changes to the Makefile to
build the headers only archive.
@richardlau
Copy link
Member Author

richardlau commented Dec 4, 2015

Retargetted to tools:.

@rvagg
Copy link
Member

rvagg commented Dec 4, 2015

@richardlau are you on Linux? the xargs thing works on OS X and that's what's used to generate the tarball. I've tried out your branch and it works fine on OS X but not on Linux, that's ... how it is and it kind of sucks but nobody has got around to fixing it yet. There's two instances of the same pattern in there.

@richardlau
Copy link
Member Author

richardlau commented Dec 4, 2015

@rvagg Yes I'm on Linux -- Didn't realize that the tarball was generated on OS X. Thanks.

@rvagg
Copy link
Member

rvagg commented Dec 4, 2015

still lgtm, needs to sit for additional reviews and possible objection for a couple of days but someone can land it after that (I'll do it if I remember but any other collaborator can)

@bnoordhuis
Copy link
Member

bnoordhuis commented Dec 4, 2015

Untested but LGTM.

@rvagg
Copy link
Member

rvagg commented Dec 4, 2015

sorry, I should have posted results of my testing on 0.12:

$ make tar-headers
/usr/local/opt/python/bin/python2.7 ./configure \
        --prefix=/ \
        --dest-cpu=x64 \
        --tag= \
         --download=all --with-intl=small-icu
creating  ./icu_config.gypi

... configure blah blah ...

creating  ./config.gypi
creating  ./config.mk
HEADERS_ONLY=1 /usr/local/opt/python/bin/python2.7 tools/install.py install 'node-v0.12.10' '/'

.. list of files blah blah ..

find node-v0.12.10/ -type l | xargs rm # annoying on windows
tar -cf node-v0.12.10-headers.tar node-v0.12.10
rm -rf node-v0.12.10
gzip -c -f -9 node-v0.12.10-headers.tar > node-v0.12.10-headers.tar.gz
rm node-v0.12.10-headers.tar
$ ls -al node-v0.12.10-headers.tar.gz
-rw-r--r--  1 rvagg  staff  444046  4 Dec 23:07 node-v0.12.10-headers.tar.gz

The full contents of the resulting file is listed here: https://gist.github.com/rvagg/e86237fb9d838ae57a62

@jbergstroem
Copy link
Member

jbergstroem commented Dec 4, 2015

LGTM

@richardlau richardlau changed the title build: backport tools/install.py for headers tools: backport tools/install.py for headers Dec 5, 2015
rvagg pushed a commit that referenced this pull request Dec 7, 2015
@richardlau @rvagg
tools: backport tools/install.py for headers
5f0b675 
Backport the tools/install.py changes from 628a3ab that were missed
when 6fb0b92 backported the corresponding changes to the Makefile to
build the headers only archive.

PR-URL: #4149
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rod Vagg <rod@vagg.org>
rvagg pushed a commit that referenced this pull request Dec 7, 2015
@richardlau @rvagg
tools: backport tools/install.py for headers
28ab7b0 
Backport the tools/install.py changes from 628a3ab that were missed
when 6fb0b92 backported the corresponding changes to the Makefile to
build the headers only archive.

PR-URL: #4149
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rod Vagg <rod@vagg.org>
@rvagg
Copy link
Member

rvagg commented Dec 7, 2015

lgtm

landed in v0.12-staging @ 5f0b675
landed in v0.10-staging @ 28ab7b0

@rvagg rvagg closed this Dec 7, 2015
@rvagg rvagg mentioned this pull request Dec 15, 2015
Closed
4 tasks
rvagg pushed a commit that referenced this pull request Feb 8, 2016
@richardlau @rvagg
tools: backport tools/install.py for headers
4f81d78 
Backport the tools/install.py changes from 628a3ab that were missed
when 6fb0b92 backported the corresponding changes to the Makefile to
build the headers only archive.

PR-URL: #4149
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rod Vagg <rod@vagg.org>
@rvagg rvagg mentioned this pull request Feb 8, 2016
Closed
rvagg pushed a commit that referenced this pull request Feb 8, 2016
@richardlau @rvagg
tools: backport tools/install.py for headers
02bc6f3 
Backport the tools/install.py changes from 628a3ab that were missed
when 6fb0b92 backported the corresponding changes to the Makefile to
build the headers only archive.

PR-URL: #4149
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rod Vagg <rod@vagg.org>
@rvagg rvagg mentioned this pull request Feb 8, 2016
Closed
jasnell pushed a commit that referenced this pull request Feb 9, 2016
@richardlau @jasnell
tools: backport tools/install.py for headers
67d4828 
Backport the tools/install.py changes from 628a3ab that were missed
when 6fb0b92 backported the corresponding changes to the Makefile to
build the headers only archive.

PR-URL: #4149
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rod Vagg <rod@vagg.org>
jasnell pushed a commit that referenced this pull request Feb 9, 2016
@richardlau @jasnell
tools: backport tools/install.py for headers
ce2a393 
Backport the tools/install.py changes from 628a3ab that were missed
when 6fb0b92 backported the corresponding changes to the Makefile to
build the headers only archive.

PR-URL: #4149
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rod Vagg <rod@vagg.org>
@gibfahn gibfahn mentioned this pull request Mar 31, 2016
Closed
4 tasks
@jbergstroem jbergstroem mentioned this pull request May 3, 2016
Closed
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
@richardlau @jBarz
tools: backport tools/install.py for headers
bb351ce 
Backport the tools/install.py changes from 628a3ab that were missed
when 6fb0b92 backported the corresponding changes to the Makefile to
build the headers only archive.

PR-URL: nodejs/node#4149
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rod Vagg <rod@vagg.org>
This was referenced Jul 17, 2021
Open
Open
Open
This was referenced Jul 24, 2021
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

tools Issues and PRs related to the tools directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@richardlau @mscdex @rvagg @bnoordhuis @jbergstroem @targos